Repository navigation
fix: send paywall_open to a paywall page reloaded after its web process dies - #545
Conversation
|
This PR does not match any of the 1 configured review trigger rule. |
Maple review🟢 Confidence 9/10 · safe to merge Tracks web-document readiness separately from paywall state and replays What was checked
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one suggestion inline to close the remaining reload gaps.
Reviewed changes
Reviewed the full diff: per-document readiness in PaywallMessageHandler, its call sites, the tests and the changelog entry.
- Readiness moved off
paywall.paywalljsVersion—isDocumentReadyis reset bydocumentWillLoad()and set byonReady, so a value left over from a dead page no longer counts as ready. - Open state tracked in the handler —
isPaywallOpenfollowspaywall_open/paywall_close, anddocumentWillLoad()queuespaywall_openagain for the next document while the paywall is open. - Hooks at every SDK-initiated load —
loadWebView(), the refresh button'sreloadWebView()andwebViewWebContentProcessDidTerminateall calldocumentWillLoad(). - Analytics unchanged —
trackOpen()still runs once per presentation, so the trackedpaywall_open,didPresentPaywallandstorage.trackPaywallOpen()are not repeated. - Tests — six new
PaywallMessageHandlerTestscover open before and after ready, a reload while open, while hidden and after close, and a page dying before it is ready.
I traced both reported paths (killed while presented → reload(); killed while preloaded → didFailToLoad → loadWebView() in viewWillAppear → trackOpen()). Both now deliver paywall_open exactly once to the new page, and a hidden preload is never told it is open.
claude-opus-5.5 ｜ 𝕏
169c8a5 to
f72f2bc
Compare
Maple reviewNothing to review Only CHANGELOG.md changed since the previous review. No new source or test changes require review.
|
…en-after-reload # Conflicts: # CHANGELOG.md
Maple review🟢 Confidence 9/10 · safe to merge Re-arms What was checked
Observability coverage: 1 of 1 changes observable
|
paywall_open was queued when the SDK started a load, so a page that reloaded itself (location.reload, the dev server's live reload) never got it, and a setup still finishing for the old page could hand the new page's paywall_open to a page that wasn't ready. A page is now ready once its own templates are in, and it is told the paywall is open at that point if it is. A setup for a page that has since been replaced is ignored. The queue this replaces was the only use of Queue, so that goes too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Maple review🟢 Confidence 8/10 · likely safe to merge
What was checked
|
There was a problem hiding this comment.
✅ No new issues found. The open-state re-arm now follows the page that pings, so the gaps from the previous review are closed.
Reviewed changes
Reviewed 84043a9, which moves delivery of paywall_open from SDK-started loads to each page's own setup.
paywall_opensent from each page's setup —didLoadWebViewsetsisDocumentReadyand, ifisPaywallOpen, passespaywall_openafter the templates. A page that reloads itself (dev-server live reload,location.reload()) now gets it without adocumentWillLoad()call.- Stale setups ignored through
documentId— bothonReadyanddocumentWillLoad()bump the counter. A setup whose id is out of date returns before it marks the page ready, so a late setup from the old document can no longer deliver into the new one. - Queue removed —
messageQueue,EnqueuedMessageand the now-unusedQueue.swift(plus its pbxproj entries) are gone..paywallOpen/.paywallCloseupdateisPaywallOpenand only reach the page once it is ready. - Tests — added a self-reload case (sent twice) and a page replaced during its setup (sent once).
I traced the three gaps from the earlier thread: self-reload, a stale drain after documentWillLoad(), and the old page pinging between load() and commit. The new page gets exactly one paywall_open in all three. Everything runs on the main actor, and the paywall_open evaluation is queued behind the templates script, so ordering holds.
claude-opus-5.5 ｜ 𝕏
…uest The straight-through test opened the paywall before the page's setup had run, so it never reached the path that sends paywall_open straight to a ready page. It now waits for the setup first. The Customer Center test checked for the catalogue request as soon as load() returned, but that request runs in the background, so it failed now and then on CI (and on develop). It now waits for the request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Maple review🟢 Confidence 10/10 · safe to merge The follow-up makes the ready-page test wait for setup and moves the catalogue assertion after the background request starts. These test-only changes are safe to merge. What was checked
|
There was a problem hiding this comment.
✅ No new issues found. The new commit only changes tests, and both changes remove a timing dependency.
Reviewed changes
Reviewed 5479093. It only touches tests.
- Straight-through test now opens a ready page.
waitForSetup(of:)polls for theuser-scalable=noscript.didLoadWebViewevaluates that script in the sameMainActor.runblock that setsisDocumentReady = true, so once the test sees it, the page is ready. The test now asserts that nothing sentpaywall_openbefore.paywallOpen. That means it covers the direct pass-through path. Before this change, the ready path could also have satisfied it. - Customer center catalogue assertion moved after the wait.
catalogueRequestedis now checked afterwaitUntil { productsMock.isHoldingCatalogue }.ProductsProviderMocksetscatalogueRequestedbefore it parks on the gate, so the assertion no longer races the background fetch.
claude-opus-5.5 ｜ 𝕏
The refresh button reset readiness before reloading. If the page's setup was still running and the reload failed, WebKit kept the old page, but it stayed marked not ready, so paywall_open never reached it. Reset when the new page commits instead, so a failed load leaves the old page as it was. The reset on web process termination is dropped: the reload that follows commits a new page, which now does the reset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Maple review🟢 Confidence 9/10 · safe to merge The change resets document readiness when navigation commits, preserving the old page's readiness during provisional reloads. The new regression test covers an open arriving between commit and the replacement page's ping; no new defects were found in the changes since the previous review. What was checked
|
There was a problem hiding this comment.
✅ No new issues found. A page is now marked not ready only when a new page commits, so a load that never commits no longer silences the old page, which is still live.
Reviewed changes
Reviewed c5305f7. It moves where the SDK marks the page not ready.
documentWillLoad()now runs fromwebView(_:didCommit:). That callback fires only for main-frame commits, so it covers the reload after the process dies, the refresh button, the initial load and in-load redirects. It skips navigations thatdecidePolicyForcancels and loads that fail before they commit.- Removed the calls in
reloadWebView()andwebViewWebContentProcessDidTerminate. The commit callback covers both.loadWebView()still callsdocumentWillLoad()itself, which keeps the hidden-preload path working: a page killed while not active doesn't reload, and it gets marked not ready at presentation. - Added a commit test.
paywallOpenWaitsForAPageThatReplacedTheOldOnecallsdidCommitonFakeWebViewdirectly. It asserts thatpaywall_openwaits for the new page's ping and is then sent once.
I traced the process-termination path. Between reload() and the commit, isDocumentReady stays true, so a paywall_open or paywall_close sent in that window goes to the dead page. That's harmless: isPaywallOpen is still updated, and the new page's ping sends paywall_open again.
claude-opus-5.5 ｜ 𝕏
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Maple reviewNothing to review Only CHANGELOG.md changed since the previous review; no new source or test changes require review, and no earlier findings remain open.
|
2 participants
The bug
Paywalls that gate their entrance on
paywall_opengo blank after iOS kills the WKWebView content process. Every framework paywall gates this way, because the SDK preloads paywalls hidden and a mount-timed animation would finish before anyone sees it.App 1022's event log shows
paywallWebviewLoad_processTerminatedfor all 11 of its paywall web views when the app returned from the background, followed about a second later bypaywallWebviewLoad_complete. The page reloaded, but it never left its pre-open state and showed only its background colour.There are two paths, and both lose
paywall_open:webViewWebContentProcessDidTerminatecallsreload(). The new document pings and gets templates, butpaywall_openis only ever sent fromtrackOpen(), which runs once per presentation. The reloaded page is never told it is on screen.didFailToLoadand callsloadWebView()at presentation, and thentrackOpen()runs. Readiness was keyed onpaywall.paywalljsVersion == nil, but that value survives from the dead page. The SDK treats the still-loading document as ready and evaluatesaccept64into it, so the message is lost.The fix
PaywallMessageHandlernow tracks readiness per document instead of per paywall:documentWillLoad()marks the page not ready. It runs when a new page commits, which covers the reload after process termination, the refresh button, and the initial load.loadWebView()also calls it as soon as it starts, so a paywall loading over a page that is still on screen doesn't sendpaywall_opento the old page. A load that fails before it commits leaves the old page, and whether it was ready, as it was.onReady(the page'sping) marks the page ready. The existing queue drain indidLoadWebViewdelivers the queuedpaywall_openafter templates.paywall_openandpaywall_closeset or clear the open state, and queue while the page is not ready.Only the message to the page is repeated. The analytics
paywall_openevent,didPresentPaywall, andstorage.trackPaywallOpen()still fire once per presentation.Why not a new event, or a fix in the framework
paywall_reloadevent would need every paywall to handle a second signal for "you are on screen". The reloaded document is a fresh page that has never been told it is open, so telling it is exactly the meaning ofpaywall_open. The framework runtime already treats a repeatedpaywall_openas idempotent.navigation.type === "reload", is unreliable. It also cannot tell a reload under a presented paywall from one under a hidden preload, where revealing would play the entrance unseen.Tests
Six new
PaywallMessageHandlerTestscover these cases:paywall_openexactly once.paywall_openonly once.PaywallMessageHandlerTests,SWWebViewLoadingHandlerTests,SWWebViewLogicTests,RawWebMessageHandlerTestsandPresentationIdTestspass on an iOS 27.0 simulator.End-to-end on a simulator
I ran the Basic example with
devServer = .defaultagainstsuperwall devserving thewith-motionexample, which gates its entrance onpaywall_open. The paywall was presented, then the web content process was killed withkill -9. I ran this once withdevelopand once with this branch, decoding the SDK's debug log for the messages posted to the page.developpaywall_opento the new pagepaywall_opensent once after the new page's pingpaywall_opensent oncepaywall_openper presentationThe first row is a second bug this fixes. Dev mode gives the paywall a non-nil empty
paywalljsVersion, so the old readiness check sentpaywall_openbefore the local page was ready. Gated paywalls served bysuperwall devnever opened. The changelog has a line for it.Not covered end to end: a process killed while the paywall is preloaded but hidden, because dev mode disables preloading. The unit tests cover that path. Not verified on a physical device.
Android
Not checked. Android forwards render-process crashes through
onRenderCrashed, but I did not trace what its handler does. Android sendspaywall_openfromtrackOpen(), including when the app returns from the background. Whether a reloaded page there gets it is still an open question.🤖 Generated with Claude Code